-
Notifications
You must be signed in to change notification settings - Fork 546
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Reactivity's effectScope
API
#212
Conversation
Not strictly against this proposal, but I don't completely understand the motivation point. I have several questions if you don't mind:
Also the code could be authored in a more simplified way, which might help with the issue: const disposables = []
const counter = ref(0)
const doubled = computed(() => counter.value * 2)
disposables.push(() => stop(doubled.effect))
disposables.push(
watchEffect(() => {
console.log(`counter: ${counter.value}`
}))
)
disposables.push(watch(doubled, () => console.log(double.value)))
disposables.forEach(f => f())
disposables = [] |
import { useMyLogic1, useMyLogic2 } from './utils'
import { useLogicFromLibrary } from 'xxx-compsoable'
const disposables = []
// each your custom logic should do the collecting in order to be disposed by the user side
const { state1, disposables: disposables1 } = useMyLogic1()
disposables.push(...disposables1)
const { state2, disposables: disposables2 } = useMyLogic2()
disposables.push(...disposables2)
// it does not collect the disposables, how do you dispose it? (note this is ok in setup)
const state3 = useLogicFromLibrary()
Thanks for the feedback, and hope I answered your concerns. |
Why is that important?
This resembles a solution to the specific component-based framework problems, but outside of these frameworks what would be the case when you need to do a lot of effect creation and disposal? Cases when we need to do a lot of cleanup are of the most interest to me. |
It's not that important, but why not if you can? Like I might be able to set up an alias and make VueUse work for @vue/lit, so I don't need to reinvent another set of utils that doing the same thing for it or yet another framework. They could even contribute back to Vue's ecosystem.
Even outside of Vue's instance this could be a problem. Maybe I would like to build a state management system my own, I might need some side-effects and to be cleaned afterward. I need to collect them. More widely, I can use it in VSCode extension (I already did), I need to watch on config changes, editor changes, file system changes... I would also need to clean them up when I no longer need some. I might use it in a server to do something like HMR which is based on local file changes. When some file gets deleted, I also need to dispose the fs watchers. There are so many possibilities for this great reactivity system. And we just got it for only a while now. |
I think this is nice because the concept is already in Vue but not cleanly abstracted. It won't get much use when
If this is accepted, the existing cleanup should 100% be based on this new API. This would give us an easy way to perform something that is really hacky today: escape component lifetime (some users have already asked how to do this in issues during the beta). setup() {
// Stopped when component is unmounted
watch(...)
// This one won't be stopped automatically
effectScope(() => {
watch(...)
})
} I'm wondering if this API could have more advanced uses by exposing the captured effects, maybe through an |
Great proposal! I love to see support for making If I think this can still be worked around though, simply by instantiating |
That's a good point. This is incentive to change the spec slightly so that nested effectScopes are not stopped by the outer one, so that Components can be implemented on top of that spec. IMHO not implementing components on top of this spec feels bad because:
It's been asked already and I know of at least two ways you can achieve this today... but both are hacky. It'd be a nice bonus if this spec allowed that in a clean way through a documented API. |
Update: const scope = effectScope(() => {
const doubled = computed(() => counter.value * 2)
watch(doubled, () => console.log(double.value))
watchEffect(() => console.log('Count: ', double.value))
})
// to dispose all effects in the scope
stop(scope) |
I really like its concept
|
What do you think of attaching effects to the scope after scope creation? That might come handy for effects that are initiated asynchronously. const scope = effectScope()
setTimeout(() => {
scope(() => {
watch(...)
})
}) Or attach by pushing: const scope = effectScope()
setTimeout(() => {
scope.push(watch(...))
}) If the scope has been destroyed the effect should not be created. |
Can't image it, can you provide some example of why to use computed outside the scope? Need some context so maybe I can think about how to change the design to fit the equipment.
Nested scope should be collected by its parent scope, but can still be disposed explicitly const scope1 = effectScope(() => {
const scope2 = effectScope(() => {
watchEffect(/* ... */)
))
stop(scope2)
})
With the last change, const scope = effectScope()
setTimeout(() => {
scope.effects.push(watch(...))
}) |
I may get you... so you mean all related logic codes are inside the scope right?
😂 my mean is how to make the child scope have a longer life than the parent scope and to be stopped outside, for example vuejs/core#1532 |
Maybe we can have a namespace so const scope1 = effectScope('namespaceA', () => {
const scope2 = effectScope('namespaceB', () => {
watchEffect(/* ... */)
))
const scope3 = effectScope('namespaceA', () => {
watchEffect(/* ... */)
))
})
stop(scope1) So scope2 is still running but scope3 is not. Also the namespace could be optional and when omited it is useing a default namespace. |
@mathe42 so in this way users should stop a scope by its name string. it looks easy but not so good for coding (personnally). and the dependency of each layer in the nested scopes is not always one-way, which may cause some problem when clean up effects under certain namespace. maybe I have the wrong idea to solve this problem |
vuejs/core#1532 seems like a reasonable use case, that would be great if we can solve it along with this RFC. I am thinking if we could add an option to it and make it not be collected by its parent. Something like: let scope2
const scope1 = effectScope(() => {
watchEffect(/* ... */)
scope2 = effectScope(() => {
watchEffect(/* ... */)
), { escaped: true })
})
stop(scope1)
// scope2 will still be alive until you dispose it |
Update:
A draft PR is filed with the feature itself implemented. vuejs/core#2195 |
@antfu if reuse it in maybe it can be like this const scope = effectScope(()=>{
return {
// some value
}
})
scope.exported // visit exported value child scopes' return value are exported by parent too. it should be a flat object. but I still don't have a good way to solve the naming conflict |
Actually, I have already made the refactor to runtime-core in the draft PR and all tests are passing ;)
Did think about that once, can't find a clean design for it though. And TBH, I don't think this feature is a must-have, you can do it like let result
const scope = effectScope(()=>{
result = {
// some value
}
})
console.log(result) |
(forwarding vuejs/core#2195 (comment)) I don't like "Forward returning values" section: const scope = effectScope(() => {
const { x } = useMouse()
const doubled = computed(() => x.value * 2)
return { x, doubled }
})
const { x, doubled } = scope
console.log(doubled.value) Here I do not like the fact that (1) what is returned by the function is affected, since it can generate some conflicts and it is generally not clear why it should be affected, and (2) there is a restriction on returning exactly the object, and it is also not clear why is it. It seems to me that it would be more elegant and simpler to return a tuple with a return value and a scope, or only a scope if the function is const [{ x, doubled }, scope] = effectScope(() => {
const { x } = useMouse()
const doubled = computed(() => x.value * 2)
return { x, doubled }
})
const scope2 = effectScope(() => {
const counter = useInterval(1_000)
watchEffect(() => console.log(counter.value))
// no return
}) It seems to me that this is simple both in implementation and in typing. |
So I did a full review of the API design while rebasing vuejs/core#2195. Since this is going to be a very low-level API, the API shape needs to be designed with implementation constraints in mind. I decided to greatly simplify the proposed API in order to obtain smaller size increase and better memory efficiency. details I will be revising this PR to reflect the updates. |
What do you think of a more fine-grained control over the scope effect execution? For example if we want to run effects on some changes (or while some flag is set), but to stop on others? const foo = ref(0)
const scope = new EffectScope(() => {
effect(() => { console.log(foo.value) })
})
foo.value = 1 // runs as usual
scope.pause()
animateValue(foo) // doesn't run during animated change
.then(() => { scope.resume() }) If we'd like to replicate current scope.destroy() // can not be resumed nor paused |
@CyberAP that's not a designed capability of the effect scope, at least in this RFC. The way it is currently designed is only meant for collecting effects so they can be stopped together. |
I revised the RFC to match the latest implementation - it is now much simpler: const scope = new EffectScope()
const returnValue = scope.run(() => { /* ... */ }
scope.stop() Please read the full rendered version. In particular, I expanded the usage example to illustrate a new pattern this would unlock: Shared Composables. |
Thanks for the reviewing, @yyx990803 ! Love the updates and now it's much more concise and elegant! Just an opinionated thought, would it better align with Vue's other APIs if we use a function to construct the instance instead of If we do, I think we could have the module implemented like this: class EffectScope {
/* ... */
}
export function effectScope() {
return new EffectScope()
}
export type { EffectScope } While we could always do it in the user land, just raising this for discussions. |
That makes sense - added the |
This RFC is now in final comments stage. An RFC in final comments stage means that: The core team has reviewed the feedback and reached consensus about the general direction of the RFC and believe that this RFC is a worthwhile addition to the framework. |
Wow this is truely amazing! This pattern is so powerful and I'm really stoked that Vue is going to provide built-ins for that. |
Looks great, I'd been thinking about this tangentially but hadn't quite put my finger on it before reading this proposal :D If you guys allow me a last minute suggestion, we could make the syntax shorter for the most common use cases by accepting a function on the constructor/factory that would get automatically run, like this: const scope = effectScope (() => { /* ... */ }); Instead of: const scope = effectScope ();
scope.run(() => { /* ... */ }) |
@udany |
About the signature of
points of view, adopting an options object as first argument, instead of a plain boolean. Current SignatureeffectScope(detached = false): EffectScope
effectScope(true) // true what? Declarative and Future ProofeffectScope(EffectScopeOptions): EffectScope
effectScope({ detached: true })
interface EffectScopeOptions {
detached?: boolean
} |
@daniele-orlando Thanks for bringing this up. Just discussed with the team, here is a short summary: The main reason we use a plain boolean for this argument is performance concern. Giving the fact that this API is an advanced and low-level API, trading a little bit of readability and extendibility allows it to save a few object allocation and restructuring cost (which could be significant on higher level when using the low-level api hundreds of times). You can find the similar reason for the optimization against the |
Hi @antfu and thanks for the answer. I see the rational behind the choice. From the public APIs domain point of view, passing an object should not be a performance problem, given that it is called only during the setup only once or few times per component and only in advanced cases. Similar use case of From the internal APIs domain point of view, passing an object can be a performance penalty and should be avoided. Given that Just wondering, I'have no knowledge of the Vue internals, so I really don't if it could be too cumbersome. |
Rendered